Skip to content

Avoid adding multiple CSRF tokens to a URL#923

Merged
markt-asf merged 7 commits intoapache:mainfrom
ChristopherSchultz:csrf-deduplicate-parameter
Jan 7, 2026
Merged

Avoid adding multiple CSRF tokens to a URL#923
markt-asf merged 7 commits intoapache:mainfrom
ChristopherSchultz:csrf-deduplicate-parameter

Conversation

@ChristopherSchultz
Copy link
Copy Markdown
Contributor

The unit tests describe the problem: if a URL already contains a CSRF token and that URL is passed through HttpServletResponse.encode(Redirect)URL, then the URL will end up with multiple instances of a CSRF token.

This patch removes those extra instances should they exist.

@ChristopherSchultz
Copy link
Copy Markdown
Contributor Author

There is a bug in this code. For the URL /foo/bar?xcsrf=&xcsrf&xcsrf&xcsrf&xcsrf=abc&xcsrf= it will enter an infinite loop.

@ChristopherSchultz
Copy link
Copy Markdown
Contributor Author

It also will incorrectly identify parameters which end with the parameter name (e.g. xcsrf).

This fixes incorrect matching of parameters whose names *end with* the CSRF token name
The code is arguably easier to read
@ChristopherSchultz
Copy link
Copy Markdown
Contributor Author

All fixed with recent commits. Ready for review.

@rmaucher
Copy link
Copy Markdown
Contributor

Remove the commented out System.outs ;)

Comment thread test/org/apache/catalina/filters/TestCsrfPreventionFilter.java
@ChristopherSchultz
Copy link
Copy Markdown
Contributor Author

Remove the commented out System.outs ;)

Done!

@markt-asf markt-asf merged commit 98187ff into apache:main Jan 7, 2026
6 checks passed
markt-asf pushed a commit that referenced this pull request Jan 7, 2026
* Avoid adding multiple CSRF tokens to a URL

* Add javadoc

* Handle special case where query-string contains a ?
markt-asf pushed a commit that referenced this pull request Jan 7, 2026
* Avoid adding multiple CSRF tokens to a URL

* Add javadoc

* Handle special case where query-string contains a ?
markt-asf pushed a commit that referenced this pull request Jan 7, 2026
* Avoid adding multiple CSRF tokens to a URL

* Add javadoc

* Handle special case where query-string contains a ?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants